Performance benchmark by dpanfilyonok · Pull Request #6 · SparseLinearAlgebra/GraphBLAS-sharp

dpanfilyonok · 2020-12-08T08:47:01Z

Proposed Changes

Changed some method signatures to match specification.
Added project for performance benchmarking.

Types of changes

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

Build and tests pass locally
I have added tests that prove my fix is effective or that my feature works (if appropriate)
I have added necessary documentation (if appropriate)

…-sharp into master

gsvgit · 2020-12-08T08:53:02Z

+                let (nrows, ncols, nnz) = float matrixInfo.[0], float matrixInfo.[1], float matrixInfo.[2]
+                let (vertices, edges) = if nrows = ncols then (nrows, nnz) else (ncols, nrows)
+                sprintf "%f" (edges / meanTime)
+            | _ -> "file`s format not supported"


It would be better to add the current file format to the message. Also, it may be useful to add supported formats.

gsvgit · 2020-12-08T08:56:21Z

+    abstract Fill: Mask1D option * int -> Scalar<'a> with set
+    abstract Fill: int * Mask1D option -> Scalar<'a> with set
+
+    abstract Mxm: Matrix<'b> -> Mask2D option -> Semiring<'a, 'b, 'c> -> Matrix<'c>


Hm... What the reason to add two new type parameters to semiring?

gsvgit · 2020-12-08T08:59:25Z

+
+    abstract Mxm: Matrix<'b> -> Mask2D option -> Semiring<'a, 'b, 'c> -> Matrix<'c>
+    abstract Mxv: Vector<'b> -> Mask1D option -> Semiring<'a, 'b, 'c> -> Vector<'c>
+    abstract EWiseAdd: Matrix<'a> -> Mask2D option -> BinaryOp<'a, 'b, 'c> -> Matrix<'a>


Why binary operation is not closed? Seems, we had discussed this point. I think semiring should have one type parameter and be closed. If it is really necessary, then one can use tuples of the required size as a domain.

Yes, it is necessary because there is a parent BFS algorithm, which use non-closed minFirst semiring. The alternative, which use embedding, doesn't seem very intuitive to me. Now, I can make the semiring closed, but this problem will require a more elegant solution in the future.

gsvgit · 2020-12-08T09:01:41Z

-    abstract EWiseAdd: Vector<'a> -> Mask1D option -> Semiring<'a> -> Vector<'a>
-    abstract EWiseMult: Vector<'a> -> Mask1D option -> Semiring<'a> -> Vector<'a>
+    abstract Vxm: Matrix<'b> -> Mask1D option -> Semiring<'a, 'b, 'c> -> Vector<'c>
+    abstract EWiseAdd: Vector<'a> -> Mask1D option -> BinaryOp<'a, 'b, 'c> -> Vector<'a>


Looks like we should use monoid instead of binaryOp, because for sparse operations we need to know an ID. Thus, it may be useful to define semiring as an extension of monoids.

gsvgit · 2020-12-08T09:07:38Z

+open GraphBLAS.FSharp
+
+[<AutoOpen>]
+module SSSP =


I think that SSSP should be defined using R^\infinite semiring. Or we assume that the distance between two nodes cannot be 0? If we use R as a domain, then distance can be equals to 0. Thus we should use R^\infinite or R+.

gsvgit · 2020-12-08T09:09:55Z

+
+[<AutoOpen>]
+module TriangleCounting =
+    // нужна проекция в инт


What does it mean?

gsvgit · 2020-12-08T09:14:00Z

+    }
+
+module BooleanSemiring =
+    let anyAll: Semiring<bool> = {


Huh... I'm confused. Here we have semiring with one type parameter, but in some files, we use semiring with three type parameters.

gsvgit · 2020-12-08T09:16:03Z

+
+module FloatSemiring =
+    let addMult: Semiring<float> = {
+        PlusMonoid = FloatMonoid.add


Mmm.... Semiring already uses Monoid in definition... Well...

gsvgit · 2020-12-08T09:16:29Z

-type Semiring<'a> = {
-    PlusMonoid: Monoid<'a>
-    Times: BinaryOp<'a, 'a, 'a>
+type Semiring<'T1, 'T2, 'TOut> = {


gsvgit · 2020-12-08T09:17:25Z

+open System
+
+type VectorType =
+    | Sparse = 0


Because they are easier to enumerate. They could be enumerated without reflection.

gsvgit

Looks good enough.

I think, we should add SSSP over R\infinite semiring in addition to the existing one. Not now, but in the nearest future. It will be useful for DU translation evaluation. It should help to estimate an overhead.
Build should be fixed.

dpanfilyonok added 25 commits October 25, 2020 02:37

Add appveyor.yml

1928754

Change semiring definition, now it has 3 domains

cf8386f

Switch tests to Expecto template

0f18072

Cleanup code, related to oclContext

3b52b27

Merge branch 'master' of https://github.com/YaccConstructor/GraphBLAS…

0514bee

…-sharp into master

Merge branch 'semiring-impl' into master

d25fd60

Change predefined semirings

1f7ff21

Add BFS impl. Remove indexers from vector

c5a1546

Changing interface

60876ea

Change interfaces, add matrix impl as record

2eb73b2

File structure cleanup

e68e74f

Fix module names to avoid errors

c4a901d

Remove inplace operations

b871dad

Update Brahma.FSharp. Add benchmark proj

426ee87

Add gh actions build and raw benchmark

78030a0

Fix gh actions build script

a41b6e0

Add custom config for benchmark

a130e56

Fix benchmark config, download method and tests

6b14acb

Fix some algorithms, add one test case

c3f8f55

Change predefined monoid and semiring names

9a138db

Add vector/matrix builders. Add comments to benchmark

0e6e92e

Add vxm correctness test

a12d7df

Fix vxm test mask

b68a2b6

Merge branch 'master' of https://github.com/YaccConstructor/GraphBLAS…

a67d0b8

…-sharp into master

Merge branch 'master' into simple-bfs-benchmark

081468f

gsvgit reviewed Dec 8, 2020

View reviewed changes

dpanfilyonok added 2 commits December 8, 2020 17:33

Fix all admonitions, except semiring closedness

4dd3fb5

Change semiring parameters count from 3 to 1

c9b07f2

dpanfilyonok requested a review from gsvgit December 8, 2020 14:43

gsvgit approved these changes Dec 8, 2020

View reviewed changes

Fix test running

205e218

gsvgit merged commit 0f7f9e1 into SparseLinearAlgebra:dev Dec 8, 2020

Conversation

dpanfilyonok commented Dec 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Types of changes

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dpanfilyonok Dec 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gsvgit left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dpanfilyonok commented Dec 8, 2020 •

edited

Loading

dpanfilyonok Dec 8, 2020 •

edited

Loading